Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gatsby-plugin-google-tagmanager): GDPR consent + default options #11379

Merged
merged 15 commits into from
Jul 9, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jan 29, 2019

Description

As of the GDPR it is a best practice to gather the user agreement on setting cookies and/or enabling tracking scripts. It's the developers responsibility to enable tracking/cookies on user consent and to remove cookies if the user declines afterwards.

But, we should give the developers options to make that possible and their jobs easier.

This PR allows two more options:

  • gdpr to disable Google Tag Manager on page load. Set to false by default.
  • gdprConsent for a global variable (inside window) that holds the users consent. Results in window['gdprConsent'] by default.

gdpr could be set by an environment variable, that is set depending on the users location (EU or not) and controlled by the ADN.

Again, it is the developers responsibility to gather the consent and set window[gdprConsent]. This value should come from a cookie and I plan to develop a plugin/component that enables the user to agree, sets both the cookie and variable, executes optional functions and renders blocked children.

The code is now wrapped inside a gtmCode() function that is needed to enable Google Tag Manager without reloading the page.

Object destructuring

This feature introduces default options by destructuring the options object. It seemed appropriate to add this with the GDPR feature.

The default value of includeInDevelopment (README) is now set too.

Related Issues

@sebastienfi uses GA (comment) in a similar way, but with a window.gdpr-agreed variable.

Carsten added 2 commits January 29, 2019 08:58
This comes with object destructuring of pluginOptions to enable default values.
@wardpeet
Copy link
Contributor

wardpeet commented Jan 31, 2019

Thanks for creating this PR, I'm just not sure this is a good fix for gdpr & googletagmanager.

Googletag manager doesn't break GDPR regulations. It's what googletag is configured to do, that might have issues with GPDR. Adding these gdpr variables in my opinion isn't a correct fix and might make this plugin a bit more complicated to use.

The correct fix is to actually push something on your DataLayer that tells gtm to load or not load scripts.

If this is hard to do I'm ok with creating a shouldLoad option that has a function to skip loading which is more general approach than just checking a global variable.

What do you think? (also want to involve @gatsbyjs/core into this discussion)

@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Jan 31, 2019
@ghost
Copy link
Author

ghost commented Jan 31, 2019

Hm, I tried the approach of using dataLayer events that trigger tags in GTM.

To control this you would need to add a script (for browser) + the dataLayer (ssr) before GTM is loaded. Time for another plugin or build this right into gatsby-plugin-google-tagmanager?

@ghost ghost removed the status: awaiting author response Additional information has been requested from the author label Jan 31, 2019
@wardpeet
Copy link
Contributor

Yes, that's true. I just don't think GDPR should be part of GTM. What do you think of creating a shouldLoad option where you can pass in a function?

@ghost
Copy link
Author

ghost commented Jan 31, 2019

The shouldLoad option would only be evaluated at build time, right?

Since the user needs to agree in the browser, this would be no solution I think.

As you said, dataLayer seems the way to go. But that requires the ability to push before GTM both at build time and in the browser.

We could add an option that takes a function which will be injected before the GTM code and allows for early dataLayer actions. Maybe the GTM code could be blocked until dataLayer is ready.

@wardpeet
Copy link
Contributor

I'll make 2 examples one with dataLayer and one with the shouldLoad function. I was thinking a function that's ran inside the browser.

@wardpeet wardpeet self-assigned this Jan 31, 2019
@wardpeet
Copy link
Contributor

@cardiv would something like this work?

@ghost
Copy link
Author

ghost commented Feb 18, 2019

Nice work @wardpeet, thank you! I am a bit confused about two code parts but this could handle some more use cases.

@wardpeet
Copy link
Contributor

@cardiv could you test if it works now. I updated the test so we catch my errors 👍

@wardpeet wardpeet removed their assignment Feb 26, 2019
@wardpeet
Copy link
Contributor

@cardiv ping :) I would love to get this merged.

wardpeet
wardpeet previously approved these changes Apr 19, 2019
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work! Function is being omitted though but the issue is not part of this PR

@wardpeet wardpeet added bot: merge on green Gatsbot will merge these PRs automatically when all tests passes status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged and removed bot: merge on green Gatsbot will merge these PRs automatically when all tests passes labels Apr 19, 2019
@freiksenet
Copy link
Contributor

There is some cleanup that needs to be done before merging. @wardpeet will address this as soon as he has time. Thank you for your patience!

@ghost ghost self-requested a review as a code owner July 9, 2019 07:19
@wardpeet wardpeet removed status: WIP status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged labels Jul 9, 2019
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's finally ship this! 🚢

@wardpeet wardpeet merged commit 3e26eeb into master Jul 9, 2019
@wardpeet wardpeet deleted the cardiv-gtm-gdpr branch July 9, 2019 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants